src/domain/relayer/solana/rpc/methods/sign_transaction.rs (1)

560-577: Same SwapInfo option-field change as above

This block mirrors the earlier Jupiter mock change; the same reasoning and suggestions apply.

Also applies to: 570-573

src/domain/transaction/evm/status.rs (1)

736-738: Tests now correctly use RpcConfig for rpc_urls

The test helpers and imports are correctly updated to construct rpc_urls as Vec<RpcConfig>, which matches the new network model and keeps status logic untouched. One minor nit: since RpcConfig is imported (Line 736), you can drop the crate::models:: qualifier in the few places that still use crate::models::RpcConfig::new(...) for consistency.

Also applies to: 803-805, 829-832, 1212-1214

src/models/transaction/repository.rs (1)

1701-1703: Test network configs aligned with RpcConfig

The EVM, Solana, and Stellar test NetworkConfigCommon fixtures now use rpc_urls: Vec<RpcConfig>, which is consistent with the new RPC model and keeps TransactionRepoModel::try_from behavior intact. If you touch this file again, you could import RpcConfig at the top and drop the repeated crate::models:: prefix in tests for slightly cleaner code.

Also applies to: 1785-1787, 1863-1865, 2125-2127

src/services/jupiter/mod.rs (1)

53-59: Making Jupiter fee fields optional is consistent and safely wired

Turning SwapInfo.fee_amount and fee_mint into Option<String> with #[serde(default)] is a good fit for handling routes where Jupiter omits these fields. The mock service and HTTP tests are correctly updated to set Some(...) values, so existing expectations still hold while allowing real responses to leave them out.

If you ever need to serialize SwapInfo back out (beyond tests), consider adding #[serde(skip_serializing_if = "Option::is_none")] on these fields to avoid emitting null keys, but that’s not required for current usage.

Also applies to: 410-419, 470-479, 684-686, 755-757

src/api/controllers/mod.rs (1)

13-18: Expose network controller and keep docs in sync

Re‑exporting the network controller from this module is the right hook for the new Networks API. As a small follow‑up, you might also add a * network – Network configuration endpoints entry to the controller list in the module docs to keep the documentation complete.

src/services/gas/fetchers/default.rs (1)

46-48: Tests correctly adapted to RpcConfig, consider importing it once

Using crate::models::RpcConfig::new(...) for rpc_urls matches the new type and is functionally fine. For readability you might add a single use crate::models::RpcConfig; at the top of the test module and use RpcConfig::new(...) everywhere instead of repeating the fully-qualified path.

Also applies to: 76-78, 105-107, 135-137, 165-167

src/domain/transaction/evm/utils.rs (1)

229-231: Unify RpcConfig imports in the test module

The switch of rpc_urls to RpcConfig is correct. You currently mix a fully-qualified path in create_standard_network with function-local use crate::models::RpcConfig; in the Arbitrum helpers. Consider a single use crate::models::RpcConfig; at the top of the test module and using RpcConfig::new(...) consistently.

Also applies to: 245-249, 262-266

src/domain/relayer/solana/rpc/methods/sign_transaction.rs (1)

327-345: SwapInfo fee fields updated correctly to Option<String>

Using Some("10".to_string()) / Some("…".to_string()) matches the new optional fee fields and keeps existing behavior for the “has fee” path. You may eventually want an additional test exercising the None case for fee_amount / fee_mint to ensure the validation and routing logic are robust when Jupiter omits these.

Also applies to: 337-340

src/repositories/network/network_redis.rs (1)

657-664: Tests updated to RpcConfig correctly; consider a shared import

The move from Vec<String> to Vec<RpcConfig> in NetworkConfigCommon.rpc_urls is correct and keeps these tests aligned with the model. To reduce repetition, you could add use crate::models::RpcConfig; at the top of the test module and use RpcConfig::new(...) instead of crate::models::RpcConfig::new(...) in both helpers.

Also applies to: 943-955

src/domain/transaction/evm/price_calculator.rs (1)

704-708: Consolidate RpcConfig imports in EVM price calculator tests

The mock networks’ rpc_urls now correctly use RpcConfig::new(...). Since multiple helpers/tests import RpcConfig locally, you can simplify by adding use crate::models::RpcConfig; once at the top of the tests module and dropping the per-function use statements.

Also applies to: 721-730, 2073-2077

src/config/config_file/network/test_utils.rs (1)

18-23: Helpers moved to RpcConfig correctly; reduce repeated local imports

All the helper constructors now build rpc_urls as Vec<RpcConfig>, which matches the updated config model and keeps the tests in sync. To declutter, you could add a single use crate::models::RpcConfig; at the top of this module and drop the per-function use lines, then keep using RpcConfig::new(...) in each helper. The JSON factory helpers that still emit URL strings are fine since they’re validating the file format, not the in-memory type.

Also applies to: 101-109, 120-128, 139-147, 160-168

src/domain/relayer/stellar/stellar_relayer.rs (1)

4-4: Sanitized Stellar RPC error handling looks solid; consider tightening test expectations

The switch to map_provider_error + sanitize_error_description with an internal tracing::error! log gives you centralized, non‑leaky RPC error responses while keeping full detail in logs. The wiring into create_error_response is correct and type‑safe.

You might optionally strengthen rpc_tests::test_rpc_provider_error to assert the mapped error code and sanitized description (e.g., that it no longer exposes the underlying ProviderError::Other message), so future changes to the sanitization layer are caught by tests rather than accidentally regressing to raw provider strings.

Also applies to: 571-590, 3067-3116

src/domain/relayer/solana/rpc/methods/transfer_transaction.rs (1)

540-543: SwapInfo fee fields correctly updated to Option<String> in tests

Using Some("...".to_string()) for fee_amount and fee_mint matches the new SwapInfo signature and keeps the Jupiter quote mocks in sync with the production type. If you don’t already have coverage elsewhere, consider adding at least one test path where these fields are None to exercise the optional case end‑to‑end.

Also applies to: 703-706

src/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rs (1)

454-457: Tests aligned with optional SwapInfo fee metadata

All updated mocks now construct SwapInfo { fee_amount: Some(...), fee_mint: Some(...) }, which is consistent with the new optional fields and keeps sign‑and‑send flows compiling and semantically correct. As with the transfer tests, you may want a negative/None case somewhere in the suite if the runtime logic branches on missing fee metadata.

Also applies to: 630-633, 809-812

docs/configuration/index.mdx (1)

68-70: Provider health configuration docs are clear and consistent

The new PROVIDER_FAILURE_THRESHOLD, PROVIDER_PAUSE_DURATION_SECS, and PROVIDER_FAILURE_EXPIRATION_SECS entries are documented coherently in the table, sample .env, and the “Provider Health Management” section, and the behavior description (failure window, pausing, recovery, and fallback) matches the variable semantics. As a small polish, consider explicitly stating whether recovery on failure expiration can happen before PROVIDER_PAUSE_DURATION_SECS elapses, to mirror the exact implementation and avoid ambiguity.

Also applies to: 105-107, 308-323

src/config/config_file/network/collection.rs (1)

832-875: New mixed-type networks deserialization test looks good; tighten variant checks

The updated test_deserialize_networks_array nicely exercises array-based deserialization for both EVM and Solana, including chain_id, rpc_urls, average_blocktime_ms, and is_testnet. To make this test more robust against accidental type regressions, consider turning the if let blocks into matches (or adding an else { panic!(...) }) so a non‑EVM/Non‑Solana variant can’t silently skip the assertions.

Example tightening for the EVM assertion

-        let evm_network = config
-            .get_network(ConfigFileNetworkType::Evm, "testnet")
-            .unwrap();
-        if let NetworkFileConfig::Evm(evm_config) = evm_network {
-            assert_eq!(evm_config.chain_id, Some(11155111));
-            assert!(evm_config.common.rpc_urls.is_some());
-            assert_eq!(evm_config.common.rpc_urls.as_ref().unwrap().len(), 2);
-        }
+        let evm_network = config
+            .get_network(ConfigFileNetworkType::Evm, "testnet")
+            .unwrap();
+        match evm_network {
+            NetworkFileConfig::Evm(evm_config) => {
+                assert_eq!(evm_config.chain_id, Some(11155111));
+                assert!(evm_config.common.rpc_urls.is_some());
+                assert_eq!(evm_config.common.rpc_urls.as_ref().unwrap().len(), 2);
+            }
+            other => panic!("Expected Evm network config, got {:?}", other),
+        }

src/models/network/evm/network.rs (1)

6-7: EvmNetwork RpcConfig migration and tests look correct

• rpc_urls: Vec<RpcConfig> plus public_rpc_urls(&[RpcConfig]) matches the new shared RPC model and the provider trait helpers.
• Tests building networks and configs with RpcConfig::new(...) correctly exercise the updated types and inheritance behavior.

If you prefer, you could pull use crate::models::RpcConfig; up to the top of the tests module instead of scattering function‑local imports, but that’s purely stylistic.

Also applies to: 14-16, 160-166, 176-191, 284-315

src/domain/relayer/evm/evm_relayer.rs (1)

59-61: RPC error sanitization and logging look good

Forwarding only (code, message) from map_provider_error plus a sanitize_error_description-derived description while logging the full ProviderError via tracing::error! cleanly separates internal detail from the external JSON‑RPC response. This aligns well with the stated RPC‑sanitization goals, and the plumbing through create_error_response is correct.

Also applies to: 492-508

src/domain/relayer/evm/rpc_utils.rs (1)

12-15: Error‑mapping re‑export and integration tests are consistent

Re‑exporting map_provider_error / sanitize_error_description from rpc_utils while delegating to crate::utils is a clean way to avoid duplication and preserve older import paths. The integration tests that build ProviderError variants and assert the resulting JSON‑RPC error codes align with the shared implementation in src/utils/error_sanitization.rs, so the wiring looks correct.

Also applies to: 72-75, 403-483

src/domain/relayer/solana/solana_relayer.rs (1)

836-848: Make insufficient‑funds token‑swap trigger best‑effort

On SolanaRpcError::InsufficientFunds, check_balance_and_trigger_token_swap_if_needed().await? means any failure in scheduling the token‑swap job bubbles up as a RelayerError instead of returning the mapped JSON‑RPC INSUFFICIENT_FUNDS error.

Consider logging failures from check_balance_and_trigger_token_swap_if_needed and still returning the JSON‑RPC error so clients always see a consistent insufficiency error, regardless of background job health.

src/utils/error_sanitization.rs (1)

40-95: Centralized provider error mapping and sanitization look solid

The mapping and sanitization functions are consistent and well‑covered by tests; the wildcard handling of Solana/selector/transport errors to INTERNAL_ERROR is reasonable for now.

If you add more ProviderError variants later, it may be worth making map_provider_error exhaustive (no _ arm) so the compiler forces you to decide on explicit codes per variant.

src/api/routes/network.rs (1)

40-46: Align route‑registration comment with actual order

The comment says “Register routes with literal segments before routes with path parameters”, but init currently registers the parameterized handlers before list_networks. Behavior is fine, but either reorder the cfg.service calls or update the comment to avoid confusion for future readers.

src/config/config_file/network/common.rs (1)

17-56: RpcConfig deserialization and merge semantics look correct

The custom deserialize_rpc_urls, the switch to Option<Vec<RpcConfig>>, and merge_optional_rpc_config_vecs all align with the documented behavior (simple + extended formats, child‑overrides semantics), and the tests thoroughly exercise validation, serialization round‑trips, and parent/child merge behavior.

You now have essentially the same deserialize_rpc_urls logic in both this module and src/models/network/request.rs. If this evolves (e.g., more fields on RpcConfig), consider extracting a shared helper to keep behavior in sync.

Also applies to: 80-108, 186-195, 813-985

src/models/network/response.rs (1)

13-113: NetworkResponse mapping from NetworkRepoModel is consistent

The flattening of common fields plus EVM/Stellar‑specific additions in From<NetworkRepoModel> for NetworkResponse matches the underlying configs, and the EVM tests verify both type‑specific and common fields.

Consider adding small tests for Solana and Stellar models to catch regressions in those branches of the match as fields evolve.

Also applies to: 115-166

src/models/network/request.rs (1)

19-80: Flexible rpc_urls request model and validation look good

The untagged RpcUrlEntry for schema purposes, the custom deserialize_rpc_urls (strings, objects, and mixed), and UpdateNetworkRequest::validate together give a nicely flexible yet validated RPC URL update surface; the tests exercise the key shapes and error cases.

You might eventually want to (a) share the deserializer with the config/common module to avoid drift, and (b) lean on DEFAULT_RPC_WEIGHT (via RpcConfig or constants) in tests instead of hard‑coding 100u8, so a future default change doesn’t require updating multiple places.

Also applies to: 82-130, 132-248

src/services/provider/evm/mod.rs (1)

201-226: New provider initialization logging is fine; consider structured fields

initialize_provider now logs the URL being initialized. To align with structured tracing patterns elsewhere, you might prefer something like info!(%url, "initializing EVM provider"); instead of interpolating into the message string, but behavior is otherwise correct.

src/models/relayer/rpc_config.rs (1)

112-148: URL validation helpers and tests are solid; minor doc-type mismatch only.

The validate_url_scheme / validate_list helpers and their tests cover the expected HTTP/HTTPS vs. invalid-scheme cases well, and the switch to eyre::Report integrates with the rest of the error stack.

Minor nit: the # Returns doc comment on validate_list still says Result<()> while the signature is Result<(), eyre::Report>. Consider updating or omitting the concrete type in the docs to avoid confusion.

Also applies to: 205-287

src/services/provider/stellar/mod.rs (2)

274-331: Exposing RPC configs via get_configs is consistent; cloning is acceptable

Adding fn get_configs(&self) -> Vec<RpcConfig> to StellarProviderTrait and delegating to the inherent StellarProvider::get_configs cleanly surfaces the selector’s current config set. Returning an owned Vec<RpcConfig> is reasonable here given the small list size and avoids lifetime complexity.

If you ever need cheaper read-only access in hot paths, consider an additional method returning a slice (&[RpcConfig]) on the concrete provider and using that where cloning isn’t needed.

Also applies to: 373-380, 502-507

---

1005-1008: Test helpers for ProviderConfig are correct but duplicated

Both create_test_provider_config helpers build ProviderConfig::new(configs, timeout, 3, 60, 60) and the updated tests exercise the new constructor paths and error cases correctly.

To avoid test-only duplication, you could expose a single helper (e.g. top-level in this module) and reuse it in concrete_tests via super::create_test_provider_config.

Also applies to: 1294-1297, 1013-1097, 1081-1097

src/config/server_config.rs (2)

31-80: Provider health config fields and env wiring are consistent

The new provider_failure_threshold, provider_pause_duration_secs, and provider_failure_expiration_secs fields are correctly:

• Documented in from_env defaults.
• Populated via the new getters.
• Exposed as plain public fields for downstream use in ProviderConfig::from_server_config.

The getters’ behavior (env → legacy env → DEFAULT_* with parse fallback) matches the pattern used elsewhere in this module.

Given these values drive RpcHealthStore behavior, you may want to document or clamp pathological cases (e.g., threshold=0 or pause/expiration=0) if those are not intended to be supported configurations.

Also applies to: 100-106, 124-132, 282-312

---

399-407: Consider extending tests to cover new provider health getters

The existing tests validate:

• Default values for provider_failure_threshold and provider_pause_duration_secs.
• from_env equivalence with individual getters (for pre-existing fields).

They don’t yet:

• Assert the default for provider_failure_expiration_secs.
• Exercise get_provider_failure_threshold, get_provider_pause_duration_secs, and get_provider_failure_expiration_secs directly or with custom/legacy env vars.

Adding a small set of assertions for these getters (including precedence of legacy RPC_* variables) would lock in the new behavior and guard future refactors.

Also applies to: 426-428, 555-608, 712-791

src/services/provider/mod.rs (1)

22-99: ProviderConfig and get_network_provider composition look correct

ProviderConfig cleanly encapsulates RPC configs, timeouts, and health parameters, and:

• from_server_config/from_env correctly translate ServerConfig fields (including ms→s for timeouts and the new provider_* health fields).
• get_network_provider selects custom vs public RPC configs as intended, errors when none are available, and then constructs a ProviderConfig via from_env for all networks before delegating to N::new_provider.

Once the public_rpc_urls recursion is fixed, this centralization should make adding future provider knobs straightforward.

If you ever need sub-second timeouts, consider switching ProviderConfig::timeout_seconds to a Duration or carrying both ms and s explicitly to avoid the integer division truncation in from_server_config.

Also applies to: 251-261, 327-346

src/services/provider/rpc_health_store.rs (1)

13-247: RpcHealthStore design and semantics look solid

The new health store:

• Uses a global Lazy<RpcHealthStore> with Arc<RwLock<...>> for safe shared state.
• Correctly prunes stale failures on both mark_failed and is_paused, caps history at threshold * 2, and updates paused_until with clear logging for first-pause vs extension.
• Cleans up metadata entries when pauses expire and no recent failures remain, avoiding unbounded map growth.
• Is thoroughly exercised by tests, including stale vs recent failure mixes, pause extension, and shared-instance behavior.

The only slightly unusual edge case is threshold == 0, which effectively means “pause on first failure” with zero retained timestamps; that’s consistent but worth documenting if users might accidentally configure it.

Also applies to: 249-705

src/services/provider/solana/mod.rs (1)

1025-1036: Test updates for ProviderConfig usage are consistent

The new create_test_provider_config helper and the refactored tests that construct SolanaProvider via:

SolanaProvider::new(create_test_provider_config(configs, timeout))

or

SolanaProvider::new_with_commitment_and_health(configs, timeout, commitment, 3, 60, 60)

are aligned with the new configuration model and correctly assert:

• Success for valid configs (single/multiple URLs).
• ProviderError::NetworkConfiguration for empty or invalid config lists.

This keeps Solana tests in sync with the EVM/Stellar provider patterns.

You could reuse create_test_provider_config in all local tests (including commitment-specific ones) to avoid hard-coding the health parameters in multiple places.

Also applies to: 1037-1111, 1140-1154

src/services/provider/rpc_selector.rs (3)

22-22: Import additional tracing macros used in the code.

The code uses tracing::warn! and tracing::debug! (e.g., line 337), but only info is imported. This requires fully-qualified paths elsewhere.

🔎 Suggested import fix

-use tracing::info;
+use tracing::{debug, info, warn};

---

167-167: Use more appropriate log level for failures.

Marking a provider as failed is a significant event that affects availability. Consider using warn! instead of info! to ensure it's visible in production logs at standard levels.

🔎 Proposed change

-        info!("Marking current provider as failed");
+        warn!("Marking current provider as failed");

---

426-429: Consider preserving error details in client initialization.

Line 428 converts the error using e.to_string(), which loses the underlying error type and source chain. For debugging, consider preserving the full error context.

💡 Alternative error handling

-        initializer(&url).map_err(|e| RpcSelectorError::ClientInitializationError(e.to_string()))
+        initializer(&url).map_err(|e| {
+            RpcSelectorError::ClientInitializationError(format!("{:#}", e))
+        })

Using {:#} preserves the error chain, providing more context for debugging.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Learnt from: LuisUrrutia
Repo: OpenZeppelin/openzeppelin-relayer PR: 427
File: src/models/network/evm/network.rs:1-1
Timestamp: 2025-08-19T14:31:09.686Z
Learning: In the OpenZeppelin relayer codebase, test helper functions may use network names (like "optimism") as string literals for creating mock networks, which are different from network tag constants (like OPTIMISM_BASED_TAG). These should not be flagged as inconsistencies when reviewing tag-related changes.

Learnt from: LuisUrrutia
Repo: OpenZeppelin/openzeppelin-relayer PR: 428
File: config/networks/bsc.json:30-32
Timestamp: 2025-08-19T10:41:31.051Z
Learning: The network inheritance mechanism in OpenZeppelin Relayer configs correctly resolves `from` references within the appropriate scope, preventing false positive collisions between networks with similar names across different blockchain types.

Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : For tests, prefer existing utils for creating mocks instead of duplicating code

Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Avoid unwrap; handle errors explicitly with Result and custom Error types for all async operations

Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Prefer defining traits when implementing services to make mocking and testing easier

Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Keep tests minimal, deterministic, and fast

Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Use assert_eq! and assert! macros appropriately

Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Use tracing for structured logging, e.g., tracing::info! for request and error logs